Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DNM] [v23.2.x] cmake tweaks for vtools trunk-based development #19886

Open
wants to merge 5 commits into
base: v23.2.x
Choose a base branch
from

Conversation

ivotron
Copy link
Member

@ivotron ivotron commented Jun 19, 2024

DNM - used for testing

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@ivotron ivotron requested a review from a team as a code owner June 19, 2024 03:10
@ivotron ivotron requested review from savex and removed request for a team June 19, 2024 03:10
ivotron and others added 5 commits July 8, 2024 23:21
Signed-off-by: Ivo Jimenez <[email protected]>
(cherry picked from commit 58ab041)
The `v_` prefixed was dropped

Signed-off-by: Ivo Jimenez <[email protected]>
(cherry picked from commit b93d1ba)
Signed-off-by: Noah Watkins <[email protected]>
(cherry picked from commit ad49bdb)
Signed-off-by: Noah Watkins <[email protected]>
(cherry picked from commit 0ddf160)
Signed-off-by: Noah Watkins <[email protected]>
(cherry picked from commit 2b73b5e)
@ivotron ivotron force-pushed the cmake-tweaks-for-vtools-trunkbased-23.2.x branch from ff70601 to 15eddb5 Compare July 9, 2024 03:21
Copy link
Contributor

@savex savex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, several nerdy comments

@@ -133,15 +133,15 @@ def _find_addr2lines(self):
# Workstation: find our build directory by searching back from binary
path_parts = self.binary.split("/")
try:
vbuild = "/".join(path_parts[0:path_parts.index("vbuild") + 3])
vbuild = "/".join(path_parts[0:path_parts.index("vbuild") + 5])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It would be good to have an example path here is a comment to help future generations.

add_custom_target(${NAME})
add_custom_command(TARGET ${NAME}
COMMAND env GOPATH=${GOPATH} ${CMAKE_GO_BINARY} build
string(REPLACE "-" "_" target_name ${NAME})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comments with example name updates and why would be good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants